Skip to content

Conversation

@MatthewLemmond
Copy link
Member

Description

Remove the endpoint variables from the module calls to Secrets Manager and KMS modules and hard-code the value in the call to be private in order to follow a secure-by-default approach without allowing changes to this.

Release required?

  • No release
  • Patch release (x.x.X)
  • Minor release (x.X.x)
  • Major release (X.x.x)
Release notes content

Removed existing_secrets_endpoint_type, allowed_network, and kms_endpoint_type to require these values always be pointing to private endpoints/network

Run the pipeline

If the CI pipeline doesn't run when you create the PR, the PR requires a user with GitHub collaborators access to run the pipeline.

Run the CI pipeline when the PR is ready for review and you expect tests to pass. Add a comment to the PR with the following text:

/run pipeline

Checklist for reviewers

  • If relevant, a test for the change is included or updated with this PR.
  • If relevant, documentation for the change is included or updated with this PR.

For mergers

  • Use a conventional commit message to set the release level. Follow the guidelines.
  • Include information that users need to know about the PR in the commit message. The commit message becomes part of the GitHub release notes.
  • Use the Squash and merge option.

alex-reiff
alex-reiff previously approved these changes Nov 5, 2024
@MatthewLemmond
Copy link
Member Author

/run pipeline

@MatthewLemmond
Copy link
Member Author

/run pipeline

Copy link
Contributor

@shemau shemau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Requires that DA uses private endpoints for SM.

But, DA has properties in ./ibm_catalog.json that will require update (removal). Which raises the issue of migration for any customer who has already deployed with public.

Copy link
Contributor

@ocofaigh ocofaigh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The upgrade test will no longer work with this change until we have support to run upgrade test in schematics (which is another week or two out).
One thing we could do is hide them in the ibm_catalog.json so catalog consumers cant use them, but we can still use them in the code. And then when we have schematics upgrade test support, we can fully remove them?

@shemau
Copy link
Contributor

shemau commented Apr 15, 2025

This PR is no longer required, since it was implemented elsewhere. A check against main branch today shows that this code is in the security-enforced DA and is not in the fully-configurable DA which is as expected.

@shemau shemau closed this Apr 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants